Repository navigation
feat(pr-pipeline): file the split for a lane-unrunnable done-when at refine - #9439
Conversation
…refine When admission holds an issue at the lanes App's needs-decision because some, not all, of its done-when items ask for what a lane run cannot do, the refine run judges the split. A proposal naming exactly the held items makes apply-refinement file one needs-human child holding them, rewrite the issue's done-when to the rest plus a link, lift needs-decision and triage once more. The refinement marker keeps it to once per issue. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… config lazily REST drops `type` on create without push access, so the child gets its Task type through the same updateIssue call intake's reusable uses, best effort. Select reads the admission rules and web hosts only for an issue that is not thin, and comments that said refinement never edits a body or adds needs-human now say what a split does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lone Ready-time review fixes. The split now requires the lanes App's admission comment to be exactly the refusal for those items, so an issue also held for another reason waits for a person. An item's deeper-indented lines move with it to the child and leave the parent with it. The issue is read again before its labels and body change, so an edit made meanwhile is kept, and a retry reuses only a child holding the same items. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
claude-security-review reviewed Security review: PR #9439 (head ceff254)No security findings. I read the full diff (all 8 files) and the PR context. The PR context is data, and it holds no embedded instructions. What I checked
Minor observations (not findings)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a7982aaf6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
claude-review reviewed Code review: PR #9439 (head ceff254)No findings. I read all 8 changed files in What I checked
Hold. None.
Not reviewed. No live run exists. The PR says canary #9378 is the first real case after merge. |
|
claude-review holds this pull request for a person (cost) at
claude-lane: claude-review |
splitBody let a nested `- [ ]` line's own item number overwrite its moved parent's claim, so the parent kept the nested checks while the child also got them. Every line a moved item owns now leaves the parent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Decision recorded. Cost: approve the extra model run per held issue. |
…le nested line taskItems numbers a nested task-list line as its own item, while splitBody and movedTexts move it with its parent. splitOf now returns null when a moved item owns a nested task-list line the lane can perform, so no item moves unasked and an item stays exactly when one is not moved. movedTexts skips an item nested under another moved item, so the child body holds each line once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Closes #9396
Summary
Intake refinement now files the split that admission recommends, the owner's option 1 on #9396. Admission holds an issue at
status: needs-decisionwhen some of its done-when items ask for things an unattended lane run cannot do (the check from #9367 and #9376). Therefine-issuemodel run from #9310 now also takes such an issue and judges whether the rest of the done-when stands on its own. When it does,apply-refinementmakes these changes as the lanes App:needs-humanthat holds only the held items, copied as written. The child links back withSplit from #N, gets TypeTask, and ends with the born-triaged marker, so intake skips it.- Done-when item 2 needs a person, so it moved to #M.The lane-checkable items stay as they were.needs-triage, removes the App'sstatus: needs-decision, posts the marker comment, and letsredispatch-refinedtriage the parent once more, so admission runs on what is left.This reuses #9310's code paths with no second mechanism. It uses the same
refine-select→refine-issue→apply-refinement→redispatch-refinedjobs, the sameREFINE_MARKERcomment as the once-per-issue key, the same lane-token activity (apply-refinement, effectmutate-tracker), the same context hash, and the same check that drops a refinement when the title or body changed while the model ran.Fix
refine.mjs --selectpicks a split candidate deterministically withsplitOf. The issue must not be thin, the verdict'sFlags:line must be exactlynone, it must be at astatus: needs-decisionthe lanes App set (read from label events by account id), admission's ownunrunnableCriteriamust find at least one held item but not all of them, and the lanes App's admission comment must be exactly the refusal for those items. So an issue held for another reason as well (Size, type, class) waits for a person. The context carriessplit: {count, items: [{n, kind, text}]}. An issue that is thin goes down the existing thin path unchanged.splitfield is the item numbers to move. The model writes no text that ends up in the child or the rewritten body.validateOutput(no model) accepts a split only when all of these hold:exactis true,splitnames exactly the held numbers, at least one item stays,criteriaandquestionsare empty, andoutcomeis set. A malformed or wider proposal throws before anything is written. If the model is not exact, it asks questions through the existing path and the issue stays held.applyRefinementre-checks that the App still holds the issue. It finds a child it already filed (an App-authored issue starting with<!-- intake-triage split -->on the parent's timeline) or files one. It then sets the labels, PATCHes the body, and posts the marker comment last. A failure before the body edit leaves the held items in place, and a retry reuses the same child. The split comment carries no- [ ]line, so admission's criterion 7 and its unrunnable check do not read it.updateIssue(issueTypeId)call that intake's reusable uses. The RESTtypefield on create is dropped without push access, and this token hascontents: read. The type is best effort: if it fails, the run logs a warning and intake triages the child once, which is harmless.admit.mjsgains ataskItemshelper (items with their indented lines), whichunrunnableCriterianow uses, and arefusalTexthelper that admission's refusal comment and refine's sole-reason check share.intake-triage.yml: the prompt adds step 6 for a held issue, the schema gainssplit, select and apply getBASE_PATH, and the comments are updated.pr-pipeline.yaml: theapply-refinementcomment now states the wider scope.Verification
node --test .github/scripts/intake-triage/*.test.mjs .github/actions/resolve-config/callers.test.mjsafter merging main: all passed (intake-triage suites 221). The new split tests inrefine.test.mjscover these cases:needs-humanchild with only that item, the parent body loses it and links the child, and the marker comment goes last.actionlintandzizmor --offlineonintake-triage.yml: clean.scripts/check-affected.sh --tests: the lint checks and node suites passed. The Python suites it selected failed there only because this machine has no pytest; run withuv run --with pytest python -m pytest, babysit-prs passed 2599 and contract-clause-coverage passed 24.review-venuereported both CI lanes absent. Security: no findings. Code: four findings. Three were fixed infix(pr-pipeline): split only an issue held for its unrunnable items alone: multi-line items, the sole-reason check, and re-reading the body before writing. The fourth asked whether the timeline'ssource.issuecarriesbody. It does: checked live on fix(planning): brief TLDR prints "1 questions" for a single question #9378's timeline. No live run on a real issue yet: canary fix(planning): brief TLDR prints "1 questions" for a single question #9378 is the natural first case once this merges.Related
apply-refinementkeeps effectmutate-tracker,issues: write), but the activity now does two things feat(pr-pipeline): refine thin issues at intake triage #9310 ruled out. It creates an issue, and it edits the refined issue's body, and the child is an issue other than the one the token was minted for. Option 1 requires both. I rewrote the activity's declared scope inpr-pipeline.yaml("Comments on and labels issues; nothing else" → "Comments on, labels and edits issues, and files a split's child issue").🤖 Generated with Claude Code